Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prep_aria: support for ARIA product v3 correction layers #1247

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

sssangha
Copy link
Contributor

@sssangha sssangha commented Aug 3, 2024

Description of proposed changes

Reminders

  • Fix #xxxx
  • Pass Pre-commit check (green)
  • Pass Codacy code review (green)
  • Pass Circle CI test (green)
  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • If modifying functionality, describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

@dbekaert
Copy link

@sssangha @yunjunz Can we move this PR along? Any updated needed @yunjunz?

@sssangha sssangha marked this pull request as ready for review August 13, 2024 20:04
@yunjunz yunjunz changed the title Add support for ARIA product v3 correction layers in prep_aria prep_aria: support for ARIA product v3 correction layers Aug 15, 2024
@yunjunz yunjunz requested review from yunjunz and mgovorcin August 15, 2024 03:37
src/mintpy/prep_aria.py Outdated Show resolved Hide resolved
Copy link
Member

@yunjunz yunjunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sssangha and @mgovorcin for the PR. Besides the comments above, could you also fix the suggestions from pre-commit and codacy checking?

src/mintpy/prep_aria.py Outdated Show resolved Hide resolved
src/mintpy/prep_aria.py Show resolved Hide resolved
src/mintpy/prep_aria.py Outdated Show resolved Hide resolved
@yunjunz
Copy link
Member

yunjunz commented Sep 9, 2024

@sssangha @dbekaert I have made some suggestions for this PR. It would be great if it can be updated and moved forward.

Copy link

codeautopilot bot commented Oct 28, 2024

PR summary

This Pull Request introduces support for ARIA product version 3 correction layers in the prep_aria module. The changes include the ability to load and process correction layers such as troposphere, ionosphere, and solid earth tides. The PR modifies existing functions to handle these new inputs and adds new functions to write correction data to HDF5 files. The update aims to enhance the processing of ARIA data by incorporating these correction layers, which can improve the accuracy of the interferometric products.

Suggestion

  1. Ensure that all new functions and modifications are well-documented, including input parameters and expected behavior, to maintain code readability and ease of use.
  2. Consider adding unit tests for the new functionalities to ensure they work as expected and to prevent future regressions.
  3. Review the handling of multiple troposphere files to ensure that the logic is robust and handles edge cases effectively.

Disclaimer: This comment was entirely generated using AI. Be aware that the information provided may be incorrect.

Current plan usage: 0.00%

Have feedback or need help?
Discord
Documentation
[email protected]

@ehavazli ehavazli requested a review from yunjunz October 30, 2024 22:01
@EJFielding
Copy link

I hope this addition can be completed soon. We can really use this ionospheric layer support to advance our NISAR Calibration and Validation activities that use ARIA S1-GUNW files.

src/mintpy/prep_aria.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mgovorcin mgovorcin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All @yunjunz suggestions have been addressed, @ehavazli or @rzinke please remove unused args from cli/prep_aria.py before the merge.

@yunjunz
Copy link
Member

yunjunz commented Nov 12, 2024

All @yunjunz suggestions have been addressed, please remove unused args from cli/prep_aria.py before the merge.

Great. I tried to test it last week, but the environment needs to be updated for the new ARIA-tools. I will try it again this weekend.

@EJFielding
Copy link

We found recently that the conda-forge ARIA-tools requires Python 3.12 but ISCE2 only allows versions up to 3.11, so it is not presently possible to make a conda environment with ISCE2, ARIA-tools, and MintPy.

@yunjunz
Copy link
Member

yunjunz commented Nov 12, 2024

Thanks for the heads-up @EJFielding, I will create a new env for this test then.

@yunjunz yunjunz requested a review from EJFielding November 12, 2024 03:14
@ehavazli
Copy link
Contributor

Hi @yunjunz and @EJFielding, what's the current status of this PR on your end?

@yunjunz
Copy link
Member

yunjunz commented Dec 19, 2024

Hi @yunjunz and @EJFielding, what's the current status of this PR on your end?

I won't have time to test the change in the coming few days. Since the PR only changes code in prep_aria, it's fine with me to merge as long as others confirm it works. @EJFielding ?

@yunjunz yunjunz requested a review from ehavazli December 19, 2024 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants